Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[improve] [pip] PIP-299: Stop dispatch messages if the individual acks will be lost in the persistent storage #21118

Merged
merged 24 commits into from
Oct 12, 2023
Merged

[improve] [pip] PIP-299: Stop dispatch messages if the individual acks will be lost in the persistent storage #21118

merged 24 commits into from
Oct 12, 2023

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Sep 4, 2023 •
edited
Loading

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode self-assigned this Sep 4, 2023
@poorbarcode poorbarcode added this to the 3.2.0 milestone Sep 4, 2023
@github-actions github-actions bot added type/PIP doc-required Your PR changes impact docs and you will update later. labels Sep 4, 2023
@poorbarcode
Copy link
Contributor Author

@asafm could you also take a look if you have time?

@poorbarcode poorbarcode mentioned this pull request Sep 4, 2023
Merged
14 tasks
codelipenghui
codelipenghui reviewed Sep 5, 2023
View reviewed changes
pip/pip-299.md Outdated Show resolved Hide resolved
codelipenghui
codelipenghui reviewed Sep 11, 2023
View reviewed changes
pip/pip-299.md Outdated Show resolved Hide resolved
@poorbarcode poorbarcode changed the title [improve] [pip] PIP-299: A limitation for the max size of cursor metadata to persist [improve] [pip] PIP-299: Stop dispatch messages if the individual acks will be lost in the persistent storage Sep 13, 2023
codelipenghui
codelipenghui reviewed Sep 19, 2023
View reviewed changes
pip/pip-299.md Outdated Show resolved Hide resolved
pip/pip-299.md Outdated Show resolved Hide resolved
pip/pip-299.md Outdated Show resolved Hide resolved
codelipenghui
codelipenghui reviewed Oct 9, 2023
View reviewed changes
pip/pip-299.md Outdated Show resolved Hide resolved
pip/pip-299.md Outdated Show resolved Hide resolved
poorbarcode and others added 2 commits October 11, 2023 10:45
@poorbarcode @codelipenghui
Update pip/pip-299.md
c7cc757 
Co-authored-by: Penghui Li <[email protected]>
@poorbarcode @codelipenghui
Update pip/pip-299.md
f7db00a 
Co-authored-by: Penghui Li <[email protected]>
@mattisonchao
Copy link
Member

Hi, @poorbarcode
Do you wanna make this configuration enabled by default in the future? if so, we can add the planning to the proposal. :)

@poorbarcode
Copy link
Contributor Author

@mattisonchao

Do you wanna make this configuration enabled by default in the future? if so, we can add the planning to the proposal. :)

That is a good suggestion, the next PIP will do these two things:

  • Provide a new config maxAckStatePersistentInBytes to instead managedLedgerMaxUnackedRangesToPersist, and deprecated managedLedgerMaxUnackedRangesToPersist. And set the default value of maxAckStatePersistentInBytes to 5m
  • Make this dispatcherPauseOnAckStatePersistentEnabled enabled by default

@mattisonchao
Copy link
Member

Okay, Sounds good to me. Thanks for your contribution. :)

@poorbarcode poorbarcode merged commit 1a352f1 into apache:master Oct 12, 2023
19 checks passed
vinayakmalik95 pushed a commit to tmdc-io/pulsar that referenced this pull request Oct 12, 2023
@poorbarcode @vinayakmalik12
[improve] [pip] PIP-299: Stop dispatch messages if the individual ack…
c3548a8 
…s will be lost in the persistent storage (apache#21118)

- DISCUSS: https://lists.apache.org/thread/2fzo1gnlyd1t5o80g1polbvbx4699r09
- VOTE: https://lists.apache.org/thread/j3k97599b3qq0lhmgyp5f93o6ny99cdt
@liudezhi2098
Copy link
Contributor

Hi @poorbarcode, do we have metrics to monitor this phenomenon for convenient monitoring and alerting?

@poorbarcode
Copy link
Contributor Author

Hi @poorbarcode, do we have metrics to monitor this phenomenon for convenient monitoring and alerting?

Only pulsar_subscription_unacked_messages is helpful for this case right now.

After the next PIP(see #21118), I will add a new metrics pulsar_subscription_metadata_size

@lhotari lhotari mentioned this pull request Sep 11, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui left review comments

@mattisonchao mattisonchao mattisonchao approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

Assignees

@poorbarcode poorbarcode

Labels
doc-required Your PR changes impact docs and you will update later. type/PIP
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@poorbarcode @mattisonchao @liudezhi2098 @codelipenghui